Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow JSON and IO in the same payload #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow JSON and IO in the same payload #1

wants to merge 1 commit into from

Conversation

jeremyisr
Copy link
Collaborator

Description

This PR aims to make it possible to multipart POST an IO and a JSON payload at the same time.
Fixes lostisland#769

Additional Notes

Example of API which requires this :
https://developer.concur.com/api-reference/receipts/endpoints.html#endpoint-post-a-receipt

cURL data and image:

curl -v -k -X POST https://us.api.concursolutions.com/receipts/v4/users/{USER ID FROM YOUR ID TOKEN} \
-H "Authorization: Bearer {YOUR ACCESS TOKEN}" \
-H "Content-Type:multipart/form-data" \
-H "link: <http://schema.concursolutions.com/{VALIDATION SCHEMA FROM SCHEMA ENDPOINT}.schema.json>;rel=describedBy" \
-F "receipt=<{PATH TO YOUR RECEIPT JSON};type=application/json" \
-F "image=@{PATH TO YOUR IMAGE};type={FILE MIME TYPE OF YOUR IMAGE}"

@jeremyisr jeremyisr force-pushed the allow-json-and-io-in-the-same-payload branch 5 times, most recently from 904693a to 27a225e Compare August 30, 2019 12:01
@jeremyisr jeremyisr force-pushed the allow-json-and-io-in-the-same-payload branch from 27a225e to ee5d658 Compare August 30, 2019 13:22
misdoro
misdoro reviewed Sep 2, 2019
View reviewed changes
lib/faraday/request/multipart.rb
end

def json?(value)
JSON.parse(value)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'm wondering if it may make sense to test if value is a string and maybe starts with { (maybe with some optional preceding whitespace?) ?

This way we will fail earlier if it is not json, and would avoid having to raise/collect stacktrace/rescue exception

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@misdoro misdoro misdoro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Issue with multipart/form data request with json body
2 participants
@jeremyisr @misdoro